Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make it easy to test-publish packages #2053

Open
wants to merge 14 commits into
base: trunk
Choose a base branch
from
Open

Conversation

psrpinto
Copy link
Member

@psrpinto psrpinto commented Dec 4, 2024

Previously, in #1924, I needed to test-publish a package without actually publishing it to NPM. At the time, I used an ad-hoc (and cumbersome) way to do so that got the job done (see PR description on #1924), but @adamziel suggested it could be useful to document such a process.

As such, this PR does three things:

  1. Introduce a local package registry that runs in the developer's machine
  2. Make it easy to switch the target registry of npm from npmjs.com to the local one, and vice-versa
  3. Document how to publish packages, both to production (npmjs.com) and to the local registry

The rendered documentation is here.

Testing instructions

Please follow the instructions in the docs to publish packages to a local registry.

Summary of changes

  • Provide a script to start the local registry
  • Provide a script to clear the local registry (removes all data used by the local registry)
  • Provide a script to enable the local registry (makes npm commands target the local registry)
  • Provide a script to disable the local registry (makes npm commands target npmjs.com)
  • Add a hook that prevents committing npmrc when the local registry is enabled
  • Provide documentation on how to publish packages

Screen captures

Local package registry

Shows the UI of the local package registry, after some packages were test-published.

Screenshot 2024-12-06 at 14 34 25

Docs

Screenshot 2024-12-06 at 16 48 18

References

@psrpinto psrpinto self-assigned this Dec 4, 2024
@psrpinto psrpinto force-pushed the test-publish-packages branch 3 times, most recently from 95d105a to e3a790e Compare December 6, 2024 13:48
@psrpinto psrpinto force-pushed the test-publish-packages branch from e3a790e to 26d20c1 Compare December 6, 2024 14:00
@psrpinto psrpinto force-pushed the test-publish-packages branch 3 times, most recently from be485bd to fad3d74 Compare December 6, 2024 16:42
@psrpinto psrpinto force-pushed the test-publish-packages branch from fad3d74 to 09076ae Compare December 6, 2024 17:00
@psrpinto psrpinto marked this pull request as ready for review December 6, 2024 17:11
@psrpinto psrpinto requested a review from a team as a code owner December 6, 2024 17:11
Copy link
Member Author

@psrpinto psrpinto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've made a number of assumptions when documenting the release process, so the docs might not be correct. I appreciate any input you might have.

Comment on lines +94 to +99
Playground's versioning strategy is to use the same version for all packages, **but** only packages that need to be released are bumped to the new version. As an example, lets consider the following scenario:

- All packages are currently at `v1.0.0`
- The following packages have changes since `v1.0.0`: `@wp-playground/cli` and `@wp-playground/remote`

When we issue a new release, only `@wp-playground/cli` and `@wp-playground/remote` will be bumped to `v1.0.1`, and all other packages will remain at `v1.0.0`.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not 100% this is correct, I appreciate any feedback you might have about this.

Copy link
Collaborator

@adamziel adamziel Dec 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is almost correct, I'd add a bit about dependencies, e.g. if we also updated @wp-playground/blueprints then @wp-playground/client would also be version bumped to consume the latest Blueprints version.


### Authenticating with npmjs.com

TODO
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I've heard someone (maybe @bgrgicak ?) mention that they were able to publish to npmjs.com from their local machine, but I'm not 100% sure of this. If you have done it before, could you comment here on how that process exactly works? Thanks in advance. I will then incorporate it into the docs.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have a npm run release command that you can run locally. AFAIR it will ask you for npmjs.org credentials.

HAS_LOCAL_REGISTRY=$(git diff --cached --name-only .npmrc | xargs grep -n "registry=http://localhost:4873/")
if [ -n "${HAS_LOCAL_REGISTRY}" ]; then
echo "The commit was rejected due to attempting to commit .npmrc with the registry set to the local registry."
Copy link
Collaborator

@adamziel adamziel Dec 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good check! I'm wondering – is it possible to use a file that's .gitignore-d instead of a versioned one like .npmrc?

Copy link
Collaborator

@adamziel adamziel Dec 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively, could this focus more on the next step? As in tell the user to run npm run local-registry:disable

npm run release
```

### Publishing to a local registry
Copy link
Collaborator

@adamziel adamziel Dec 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

H2 perhaps? No strong opinions

Suggested change
### Publishing to a local registry
## Publishing to a local registry

@@ -13,10 +13,15 @@
"format": "nx format",
"format:uncommitted": "nx format --fix --parallel --uncommitted",
"lint": "nx run-many --all --target=lint",
"local-registry:clear": "rm -rf tmp/verdaccio",
"local-registry:enable": "./tools/scripts/local-registry.mjs enable",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would there be any downside to running npm config set directly?

@adamziel
Copy link
Collaborator

Thank you @psrpinto! It looks great overall, I just left a few minor notes

@psrpinto
Copy link
Member Author

Thanks for reviewing @adamziel!, Your notes are on point, I will work on addressing them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Reviewed
Development

Successfully merging this pull request may close these issues.

2 participants